ethernet: stm32: add a alternative HAL_ETH_Init for V1 api #88483

maass-hamburg · 2025-04-11T07:51:24Z

add a alternative HAL_ETH_Init for V1 api without phy
configuration, as this is already been done outside of
the ethernet driver.

Signed-off-by: Fin Maaß [email protected]

erwango · 2025-04-11T14:50:57Z

drivers/ethernet/eth_stm32_hal.c

+#if defined(CONFIG_ETH_STM32_HAL_API_V1)
+#define ETH_TIMEOUT_SWRESET 500U
+
+HAL_StatusTypeDef HAL_ETH_Init_alt(ETH_HandleTypeDef *heth)


It is fine not using HAL_ETH_Init, but please don't reimplement a HAL function and instead implement as much as possible a zephyr function:
Don't call it HAL_... but use zephyr function name
Don't return HAL type and take dev as argument.
Access ETH registers instead of (heth->Instance)->
....
Likely the 2 only pure HAL things that should remain are the State and Lock updates for further HAL operations.

clamattia

Side note. Why is eth_stm32_init_v1_api happening at driver init. And the equivalent for v2 at api->init (called by net core)?

maass-hamburg · 2025-04-17T07:48:42Z

Side note. Why is eth_stm32_init_v1_api happening at driver init. And the equivalent for v2 at api->init (called by net core)?

v2 got moved there in #86621. the phy specific code from that than got removed in my #87593. Maybe it can be moved back? @marwaiehm-st what do you think, it was your pr?

marwaiehm-st · 2025-04-22T08:49:19Z

Side note. Why is eth_stm32_init_v1_api happening at driver init. And the equivalent for v2 at api->init (called by net core)?

v2 got moved there in #86621. the phy specific code from that than got removed in my #87593. Maybe it can be moved back? @marwaiehm-st what do you think, it was your pr?

Yes, I moved the V2 to eth_iface_init in my proposition of autonegotiation because I needed the Ethernet interface to be properly initialized to read the correct value from the register using 'HAL_ETH_ReadPHYRegister'. However, now with #87593 we can moved back to eth_initialize with the V1.

maass-hamburg · 2025-04-23T15:40:38Z

Side note. Why is eth_stm32_init_v1_api happening at driver init. And the equivalent for v2 at api->init (called by net core)?

v2 got moved there in #86621. the phy specific code from that than got removed in my #87593. Maybe it can be moved back? @marwaiehm-st what do you think, it was your pr?

Yes, I moved the V2 to eth_iface_init in my proposition of autonegotiation because I needed the Ethernet interface to be properly initialized to read the correct value from the register using 'HAL_ETH_ReadPHYRegister'. However, now with #87593 we can moved back to eth_initialize with the V1.

added a commit to move it back

clamattia

Thank you for moving the v1 and v2 functions closer together. I have added another small hint. I am not familiar enough with the hal to review the eth_stm32_init_v1_api. From what I can see looks good.

drivers/ethernet/eth_stm32_hal.c

maass-hamburg · 2025-05-13T08:37:31Z

ping @erwango

maass-hamburg · 2025-06-06T09:50:01Z

@erwango please take a look

erwango · 2025-06-06T13:00:50Z

@erwango please take a look

I'll take time next week. Don't hesitate to ping me again.

etienne-lms

Some minor comments

etienne-lms · 2025-06-10T09:02:52Z

drivers/ethernet/eth_stm32_hal.c

+	/* Get the ETHERNET MACMIIAR value, this is responsible for mdio, so save it to restore it
+	 * later
+	 */
+	tmpreg1 = (heth->Instance)->MACMIIAR;


can remove the parentheses.
Ditto at lines 880, 883 and 892.

etienne-lms · 2025-06-10T09:03:20Z

drivers/ethernet/eth_stm32_hal.c

+	uint32_t tmpreg1 = 0U;
+
+	if (heth->State == HAL_ETH_STATE_RESET) {
+		/* Allocate lock resource and initialize it */


Allocate does not really apply here. Maybe simply remove the inline comment.

etienne-lms · 2025-06-10T09:08:34Z

drivers/ethernet/eth_stm32_hal.c

+	if (WAIT_FOR((((heth->Instance)->DMABMR & ETH_DMABMR_SR) == (uint32_t)RESET),
+		     ETH_TIMEOUT_SWRESET, NULL) == false) {


Could remove some parentheses (easier to read)

Suggested change

if (WAIT_FOR((((heth->Instance)->DMABMR & ETH_DMABMR_SR) == (uint32_t)RESET),

ETH_TIMEOUT_SWRESET, NULL) == false) {

if (!WAIT_FOR((heth->Instance->DMABMR & ETH_DMABMR_SR) == (uint32_t)RESET,

ETH_TIMEOUT_SWRESET, NULL)) {

etienne-lms · 2025-06-10T09:11:22Z

drivers/ethernet/eth_stm32_hal.c

+		return -ETIMEDOUT;
+	}
+
+	(heth->Instance)->MACMIIAR = (uint32_t)tmpreg1;


cast not needed, tmpreg1 is already of type uint32_t.

sonarqubecloud · 2025-06-10T10:03:23Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

maass-hamburg · 2025-06-17T09:17:30Z

@erwango please take a look

I'll take time next week. Don't hesitate to ping me again.

ping @erwango

maass-hamburg · 2025-07-25T06:11:29Z

@erwango ping

erwango · 2025-07-25T07:53:51Z

@marwaiehm-st Could you have a cross check and perform non regression on the changes ?

marwaiehm-st · 2025-07-30T09:24:59Z

drivers/ethernet/eth_stm32_hal.c

+	heth->Instance->MACMIIAR = tmpreg1;
+
+	/* Config MAC and DMA */
+	ETH_MACDMAConfig(heth, ETH_SUCCESS);


The function ETH_MACDMAConfig is defined as static in the STM32 HAL module.

static void ETH_MACDMAConfig(ETH_HandleTypeDef *heth, uint32_t err);

Using ETH_MACDMAConfig here will cause a build error (undefined reference) because it is not accessible from other modules.

as changing the hal is now necessary, it was easier, to just remove the phy init and config code from the v1 HAL_ETH_Init

the phy is already configured by the phy driver in zephyr and we don't want to overwrite it in the HAL_ETH_Init function in the stm32 HAL. Signed-off-by: Fin Maaß <[email protected]>

github-actions · 2025-08-01T09:06:29Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_stm32	zephyrproject-rtos/hal_stm32@`1e75326`	zephyrproject-rtos/hal_stm32#300	zephyrproject-rtos/hal_stm32#300/files

⛔ DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

sonarqubecloud · 2025-08-01T09:20:42Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

erwango reviewed Apr 11, 2025

View reviewed changes

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch from 1b6ba6d to d3d1df1 Compare April 16, 2025 15:23

maass-hamburg marked this pull request as ready for review April 16, 2025 15:42

github-actions bot added platform: STM32 ST Micro STM32 area: Ethernet labels Apr 16, 2025

github-actions bot requested review from decsny, djiatsaf-st, FRASTM, gautierg-st, GeorgeCGV, lmajewski, marwaiehm-st, mathieuchopstm and pdgendt April 16, 2025 15:43

github-actions bot assigned erwango Apr 16, 2025

maass-hamburg mentioned this pull request Apr 16, 2025

drivers: ethernet: phy: API add ability to read, whether auto-negotiation is enabled or whether a fixed link is used #88443

Open

clamattia reviewed Apr 17, 2025

View reviewed changes

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch 2 times, most recently from 6f0eaf8 to 6fe7d22 Compare April 23, 2025 12:26

maass-hamburg requested review from clamattia and erwango April 23, 2025 15:40

clamattia previously approved these changes Apr 24, 2025

View reviewed changes

drivers/ethernet/eth_stm32_hal.c Outdated Show resolved Hide resolved

marwaiehm-st previously approved these changes Apr 25, 2025

View reviewed changes

maass-hamburg dismissed stale reviews from marwaiehm-st and clamattia via 3cdcaca May 5, 2025 07:45

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch 2 times, most recently from 3cdcaca to 0c4e0e1 Compare May 5, 2025 10:21

maass-hamburg requested review from clamattia and marwaiehm-st May 13, 2025 08:36

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch from 0c4e0e1 to d83736d Compare June 10, 2025 06:36

github-actions bot requested a review from etienne-lms June 10, 2025 06:37

etienne-lms reviewed Jun 10, 2025

View reviewed changes

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch from d83736d to 0cc6663 Compare June 10, 2025 09:38

maass-hamburg requested a review from etienne-lms June 10, 2025 16:11

marwaiehm-st reviewed Jul 30, 2025

View reviewed changes

ethernet: stm32: remove phy init and config from HAL_ETH_Init

6df0e3f

the phy is already configured by the phy driver in zephyr and we don't want to overwrite it in the HAL_ETH_Init function in the stm32 HAL. Signed-off-by: Fin Maaß <[email protected]>

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch from 0cc6663 to b0d7838 Compare August 1, 2025 09:04

github-actions bot added manifest manifest-hal_stm32 DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Aug 1, 2025

maass-hamburg force-pushed the stm32_eth_add_init_v1 branch from b0d7838 to 6df0e3f Compare August 1, 2025 09:17

		if (WAIT_FOR((((heth->Instance)->DMABMR & ETH_DMABMR_SR) == (uint32_t)RESET),
		ETH_TIMEOUT_SWRESET, NULL) == false) {

ethernet: stm32: add a alternative HAL_ETH_Init for V1 api #88483

Are you sure you want to change the base?

ethernet: stm32: add a alternative HAL_ETH_Init for V1 api #88483

Conversation

maass-hamburg commented Apr 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clamattia left a comment

Choose a reason for hiding this comment

Uh oh!

maass-hamburg commented Apr 17, 2025

Uh oh!

marwaiehm-st commented Apr 22, 2025

Uh oh!

maass-hamburg commented Apr 23, 2025

Uh oh!

clamattia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maass-hamburg commented May 13, 2025

Uh oh!

maass-hamburg commented Jun 6, 2025

Uh oh!

erwango commented Jun 6, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 10, 2025

Quality Gate passed

Uh oh!

maass-hamburg commented Jun 17, 2025

Uh oh!

maass-hamburg commented Jul 25, 2025

Uh oh!

erwango commented Jul 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 1, 2025

Quality Gate passed

Uh oh!

Uh oh!

github-actions bot commented Aug 1, 2025 •

edited

Loading